-
Notifications
You must be signed in to change notification settings - Fork 9
2493 fix callback bugs for reducing in lb subcomponent #2494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2493 fix callback bugs for reducing in lb subcomponent #2494
Conversation
|
diff --git a/src/vt/pipe/callback/cb_union/cb_raw_base.impl.h b/src/vt/pipe/callback/cb_union/cb_raw_base.impl.h
index 4d131fc77..bf959acfb 100644
--- a/src/vt/pipe/callback/cb_union/cb_raw_base.impl.h
+++ b/src/vt/pipe/callback/cb_union/cb_raw_base.impl.h
@@ -133,12 +133,9 @@ void CallbackTyped<Args...>::send(Params&&... params) {
// wrong ParamMsg will be cast to which requires serialization, leading to
// a failure.
if constexpr (sizeof...(Params) == sizeof...(Args) + 1) {
- using MsgT = messaging::ParamMsg<
- std::tuple<
- std::decay_t<std::tuple_element_t<0, std::tuple<Params...>>>,
- std::decay_t<Args>...
- >
- >;
+ using MsgT = messaging::ParamMsg<std::tuple<
+ std::decay_t<std::tuple_element_t<0, std::tuple<Params...>>>,
+ std::decay_t<Args>...>>;
auto msg = vt::makeMessage<MsgT>();
msg->setParams(std::forward<Params>(params)...);
CallbackRawBaseSingle::sendMsg<MsgT>(msg);
|
Pipelines resultsvt-build-amd64-ubuntu-20-04-gcc-10-openmpi-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-clang-9-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-alpine-3-16-clang-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-24-04-gcc-14-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-gcc-9-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-24-04-clang-16-vtk-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-clang-12-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-gcc-12-vtk-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-24-04-clang-16-zoltan-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-gcc-12-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-clang-11-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-clang-14-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-clang-15-cpp-cxx20 Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-gcc-11-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-gcc-9-ldms-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-24-04-gcc-13-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-gcc-10-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-24-04-clang-18-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-clang-10-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-24-04-clang-17-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-22-04-clang-13-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-gcc-9-cuda-12-2-0-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-gcc-9-cuda-11-4-3-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) vt-build-amd64-ubuntu-20-04-icpx-cpp Build for 9074bcd (2025-11-04 22:35:05 UTC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2494 +/- ##
===========================================
- Coverage 88.55% 88.47% -0.08%
===========================================
Files 728 728
Lines 31516 30909 -607
===========================================
- Hits 27910 27348 -562
+ Misses 3606 3561 -45
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| using MsgT = typename Trait::MsgT; | ||
| auto msg = makeMessage<MsgT>(std::forward<Params>(params)...); | ||
| sendMsg(msg.get()); |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing template parameter for sendMsg call. This should be CallbackRawBaseSingle::sendMsg<MsgT>(msg) to match the pattern used elsewhere in the function (lines 144, 149) and maintain consistency with the explicit template parameter requirement throughout this PR.
| sendMsg(msg.get()); | |
| CallbackRawBaseSingle::sendMsg<MsgT>(msg.get()); |
| // If we use the type for Params to send, it's possible that we have a | ||
| // type mismatch in the actual handler type. A possible edge case is when | ||
| // a char const* is sent, but the handler is a std::string. In this case, | ||
| // the ParamMsg will be cast incorrectly during the virual dispatch to a |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'virual' to 'virtual'.
| // the ParamMsg will be cast incorrectly during the virual dispatch to a | |
| // the ParamMsg will be cast incorrectly during the virtual dispatch to a |
Fixes #2493